Skip to content

fix(NcRichText): render invalid relative markdown links as external#8397

Open
Antreesy wants to merge 5 commits intofix/8397/nested-linksfrom
fix/noid/external-links
Open

fix(NcRichText): render invalid relative markdown links as external#8397
Antreesy wants to merge 5 commits intofix/8397/nested-linksfrom
fix/noid/external-links

Conversation

@Antreesy
Copy link
Copy Markdown
Contributor

@Antreesy Antreesy commented Apr 7, 2026 •
edited
Loading

☑️ Resolves

fix(autolink): fix rendering of nested markdown link with

  • with autolink + useMarkdown props [href2](href1) syntax is rendered as <a href1><a href2>href2</a></a> instead of href2 being a plain text
🏚️ Before 🏡 After
image image

fix(NcRichText)!: render all non-resolved links as external

  • this covers:
    • external links (as before)
    • internal absolute links, but from another app (as before)
    • and internal relative links, that do not have a router match for current app (new)
      • possibly a breaking change?
      • this is likely an invalid user input, it shouldn't unload the page and destroy an app

🚧 Tasks

  • Consult other teams for using of relative links, whether it's breaking for them

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable
  • 2️⃣ Backport to stable8 for maintained Vue 2 version or not applicable

@Antreesy Antreesy added this to the 9.6.0 milestone Apr 7, 2026
@Antreesy Antreesy requested a review from ShGKme April 7, 2026 11:46
@Antreesy Antreesy self-assigned this Apr 7, 2026
@Antreesy Antreesy added 3. to review Waiting for reviews feature: richtext Related to the richtext component labels Apr 7, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 7, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 63.63636% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.66%. Comparing base (7fda70f) to head (ad1d51c).

Files with missing lines Patch % Lines
src/components/NcRichText/NcRichText.vue 50.00% 4 Missing ⚠️
Additional details and impacted files 
@@                    Coverage Diff                    @@
##           fix/8397/nested-links    #8397      +/-   ##
=========================================================
- Coverage                  54.69%   54.66%   -0.04%     
=========================================================
  Files                        104      105       +1     
  Lines                       3406     3412       +6     
  Branches                     994      996       +2     
=========================================================
+ Hits                        1863     1865       +2     
- Misses                      1304     1308       +4     
  Partials                     239      239

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Antreesy Antreesy modified the milestones: 9.6.0, 9.7.0 Apr 15, 2026
susnux
susnux approved these changes Apr 19, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

otherwise fine :)

Comment thread src/components/NcRichText/NcRichTextExternalLink.vue Outdated
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/autolink.ts
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/NcRichTextExternalLink.vue
ShGKme
ShGKme reviewed Apr 20, 2026
View reviewed changes
Comment thread src/components/NcRichText/NcRichTextExternalLink.vue Outdated
@ShGKme
Copy link
Copy Markdown
Contributor

ShGKme commented Apr 20, 2026

fix(NcRichText)!: render all non-resolved links as external

  • this covers:

    • external links (as before)

    • internal absolute links, but from another app (as before)

    • and internal relative links, that do not have a router match for current app (new)

      • possibly a breaking change?
      • this is likely an invalid user input, it shouldn't unload the page and destroy an app

I'd even consider forbidding such links.

F relative link can even be a link to a completely different service if Nextcloud is hosted on a non-empty base URL.

@ShGKme ShGKme added the bug Something isn't working label Apr 20, 2026
@Antreesy Antreesy mentioned this pull request Apr 21, 2026
Open
3 tasks
Antreesy added 5 commits April 21, 2026 13:38
@Antreesy
fix: extract NcRichTextExternalLink component
58aeaa3 
- add noExtDecoration prop for reusing the component

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy
fix(NcRichText)!: render all non-resolved links as external
9e333c6 
- this covers external links, internal absolute links, and internal relative links, that do not have a router match for current app
- possibly a breaking change

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy
fixup! fix: extract NcRichTextExternalLink component
b9c81c0 
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy
fixup! fix(NcRichText)!: render all non-resolved links as external
cdf9171 
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy
fix(NcRichText)!: do not render non-resolved relative links
ad1d51c 
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy force-pushed the fix/noid/external-links branch from 6b21c2b to ad1d51c Compare April 21, 2026 12:24
@Antreesy Antreesy changed the base branch from main to fix/8397/nested-links April 21, 2026 12:26
@Antreesy Antreesy requested review from ShGKme and susnux April 21, 2026 12:26
@Antreesy
Copy link
Copy Markdown
Contributor Author

Antreesy commented Apr 21, 2026

I'd even consider forbidding such links.
F relative link can even be a link to a completely different service if Nextcloud is hosted on a non-empty base URL.

See last commit for the change

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mejo- mejo- Awaiting requested review from mejo-

@DorraJaouad DorraJaouad Awaiting requested review from DorraJaouad

@susnux susnux Awaiting requested review from susnux

@ShGKme ShGKme Awaiting requested review from ShGKme

Assignees

@Antreesy Antreesy

Labels

3. to review Waiting for reviews bug Something isn't working feature: richtext Related to the richtext component

Projects

None yet

Milestone

9.7.0

Development

Successfully merging this pull request may close these issues.

3 participants

@Antreesy @ShGKme @susnux